Skip to content

Conversation

@bizzappdev
Copy link
Contributor

Dependency PR for module account_banking_mandate_sale #1456

@xaviedoanhduy
Copy link
Contributor

hello @bizzappdev, do you have any plans to continue this work?

If you have any problems, I am here to help you.

@bizzappdev
Copy link
Contributor Author

hello @bizzappdev, do you have any plans to continue this work?

If you have any problems, I am here to help you.

@xaviedoanhduy Yes, we are planning to continue the work. The module is ready, but for some strange reason (it might be a dependent module from test-requirement.txt), it is causing the test cases to fail. We are investigating in depth.

@xaviedoanhduy
Copy link
Contributor

xaviedoanhduy commented May 16, 2025

as you said, it happens in 2 modules account_payment_mode and account_payment_sale, have a look in the PR that I tried to fix it: xaviedoanhduy#4

I think when this PR #1467 is merged the CI will be green when you rebase with the original branch

_inherit = "sale.order"

@api.depends(
"partner_id", "partner_invoice_id", "partner_shipping_id", "payment_mode_id"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need to copy all the dependent fields in this decorator, just add new fields that don't appear in the original function and the ORM will automatically join them

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 453d99e to 68fefb6 Compare May 16, 2025 12:11
@xaviedoanhduy
Copy link
Contributor

hello @bizzappdev, #1467 has been merged, is this PR ready?

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 68fefb6 to 93b12ee Compare May 19, 2025 05:01
@bizzappdev bizzappdev marked this pull request as ready for review May 19, 2025 08:00
@bizzappdev
Copy link
Contributor Author

@xaviedoanhduy Yes, it is now ready for review, but the dependency Module migration PR is still there, which will restrict merging this PR.

Copy link
Contributor

@xaviedoanhduy xaviedoanhduy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the work on this! The proposed changes make sense and look good to me overall 👍

Approved ✅

help="The partner of the sales in which odoo will search for the mandate"
title="The contact of this company in which odoo will search for the mandate on sales. 
-Customer Mandate: Odoo will look the mandate in the sale partner, whether is an individual or the company. 
-Commercial Customer Mandate: Odoo will look the mandate in the sale partner company. 
-Invoice Address Mandate: Odoo will look the mandate in the sale invoice address. 
-Delivery Address Mandate: Odoo will look the mandate in the sale delivery address. 
-False: Odoo will use the first mandate he founds for the partner company. Odoo will also use this option if no default mandate is found in the partner of the above options"
>
<field name="sale_default_mandate_contact" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<field name="sale_default_mandate_contact" />
<field name="sale_default_mandate_contact" class="oe_inline" />

before

image

after

image

<field name="model">res.partner</field>
<field
name="inherit_id"
ref="account_payment_partner.view_partner_property_form"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
ref="account_payment_partner.view_partner_property_form"
ref="account_banking_mandate_contact.view_partner_property_form"

I think this will look clearer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both changes are applied.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 93b12ee to 8fc7933 Compare May 22, 2025 08:17
@javierizaca
Copy link

Hello @bizzappdev,

Do you plan to continue this work? I see that the dependency module has already been merged.

@bizzappdev bizzappdev force-pushed the 18.0-mig-account_banking_mandate_sale_contact-BAD branch from 8fc7933 to bede0e5 Compare September 18, 2025 13:59
@bizzappdev
Copy link
Contributor Author

Yes sure. Also the test requirement file has been taken care of.

@javierizaca
Copy link

Yes sure. Also the test requirement file has been taken care of.

Thanks for working on this!

Copy link

@javierizaca javierizaca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, it works as expected.

Thank you very much for your work, @bizzappdev

@pedrobaeza
Copy link
Member

/ocabot migration account_banking_mandate_sale_contact
/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-1465-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot added this to the 18.0 milestone Oct 30, 2025
@OCA-git-bot OCA-git-bot mentioned this pull request Oct 30, 2025
19 tasks
@OCA-git-bot OCA-git-bot merged commit 5825843 into OCA:18.0 Oct 30, 2025
6 of 7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at be28263. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants